Skip to content

add invalid and error details to vcard import report#9605

Merged
pabzm merged 4 commits intoroundcube:masterfrom
johndoh:vcard_import
Dec 2, 2024
Merged

add invalid and error details to vcard import report#9605
pabzm merged 4 commits intoroundcube:masterfrom
johndoh:vcard_import

Conversation

@johndoh
Copy link
Copy Markdown
Contributor

@johndoh johndoh commented Aug 25, 2024

for #9591

currently there are 2 silent error cases on vcard import, invalid (fails roundcube contact validation) and error (its valid but i didnt insert for some reason e.g. plugin prevented it) this adds messages about those to the screen.

Untitled-1

Copy link
Copy Markdown
Member

@pabzm pabzm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the contribution! I really appreciate your attention to issues related to "your" topic!

Comment thread program/actions/contacts/import.php Outdated
Copy link
Copy Markdown
Member

@pabzm pabzm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, thank you!

@pabzm pabzm requested a review from alecpl September 16, 2024 13:16
Copy link
Copy Markdown
Member

@alecpl alecpl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved with some comments.

Comment thread program/localization/en_US/messages.inc Outdated
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should probably get rid of HTML tags from all these three messages.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think removing the <b> tags would be a breaking change because the formatting should be moved to the skin to do this properly. I can work on this in a separate PR.

Comment thread program/actions/contacts/import.php Outdated
@pabzm
Copy link
Copy Markdown
Member

pabzm commented Nov 11, 2024

@alecpl I'll merge this in one week if nothing happens until then, since you left your approval already and the only open topic is very minor.

@alecpl
Copy link
Copy Markdown
Member

alecpl commented Nov 19, 2024

Feel free to merge, close the related ticket (assigning milestone) if appropriate and update the changelog.

@pabzm
Copy link
Copy Markdown
Member

pabzm commented Nov 20, 2024

@johndoh Would you add a line to the Changelog? (I can do it as well but would prefer to have it as part of the PR.)

@pabzm
Copy link
Copy Markdown
Member

pabzm commented Nov 20, 2024

/remind me to merge on Monday

@github-actions
Copy link
Copy Markdown

@pabzm set a reminder for 11/25/2024

@johndoh
Copy link
Copy Markdown
Contributor Author

johndoh commented Nov 20, 2024

I've added a changelog entry. If you don't like the text or its in the wrong place its no problem.

@github-actions
Copy link
Copy Markdown

👋 @pabzm, merge

@github-actions github-actions Bot removed the reminder label Nov 25, 2024
@pabzm pabzm merged commit 0c57564 into roundcube:master Dec 2, 2024
@pabzm
Copy link
Copy Markdown
Member

pabzm commented Dec 2, 2024

@johndoh Thanks again for your contribution! 🙏

@pabzm pabzm added this to the 1.7-beta milestone Dec 2, 2024
@johndoh johndoh deleted the vcard_import branch December 4, 2024 13:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants